Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Paths for trips #724

Merged
merged 17 commits into from
Jan 29, 2025
Merged

Paths for trips #724

merged 17 commits into from
Jan 29, 2025

Conversation

Freika
Copy link
Owner

@Freika Freika commented Jan 24, 2025

Added

  • Enabled Postgis extension for PostgreSQL.
  • Trips are now store their paths in the database independently of the points.
  • Trips are now being rendered on the map using their precalculated paths instead of list of coordinates.

Changed

@Freika Freika changed the title Feature/tracks Paths for trips Jan 29, 2025
@Freika Freika changed the base branch from master to dev January 29, 2025 14:17
@Freika Freika merged commit 375c50d into dev Jan 29, 2025
4 checks passed
@Freika Freika deleted the feature/tracks branch January 29, 2025 14:17

class AddPathToTrips < ActiveRecord::Migration[8.0]
def change
add_column :trips, :path, :line_string, srid: 3857
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think 3857 is the correct value to use. I think it should be 4326.
Ref: https://gis.stackexchange.com/a/380296/55872

Not sure it matters right now (i.e. it probably works with 3857 too for now), but it might cause issues later, if you try to use PostGIS functions like ST_Contains().

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not an expert in GIS, but I did my small research and came to a conclusion that 3857 serves well in case of flat planar projections, specifically for web mapping. But as I said, not an expert, so if you could provide pro/contra for 4326, I may consider changing it in the future. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants